Skip to content

[FR-126] Rollback#274

Open
eniko-dif wants to merge 6 commits intotemporalio:mainfrom
eniko-dif:eniko/feature-rollback
Open

[FR-126] Rollback#274
eniko-dif wants to merge 6 commits intotemporalio:mainfrom
eniko-dif:eniko/feature-rollback

Conversation

@eniko-dif
Copy link
Copy Markdown

@eniko-dif eniko-dif commented Apr 14, 2026

What was changed

Add a separate rollback config to the deployment specification that can be used to have different rollback behavior from the rollout one. It supports AllAtOnce or Progressive and does not have a gate. Uses the LastCurrentTime of a deployment to decide if the target version supposed to be rolled out or rolled back.

Why?

Open feature issue: #126

Checklist

  1. Closes [Feature Request] Rollback mode #126

  2. How was this tested:

Unit and integration tests (ongoing actual test with local setup).

  1. Any docs updates needed?

Yes, but needs to be decided where.

@eniko-dif eniko-dif requested review from a team and jlegrone as code owners April 14, 2026 15:35
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 14, 2026

CLA assistant check
All committers have signed the CLA.

Comment thread api/v1alpha1/worker_types.go
Comment thread api/v1alpha1/worker_types.go Outdated
Comment on lines +370 to +371
// RollbackProgressive gradually ramps traffic back to the previous version.
RollbackProgressive DefaultVersionRollbackStrategy = "Progressive"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we have a progressive rollout mode, we should also require that there be at least 1 step defined. It could be reasonable to assume that choosing the progressive rollout mode without also adding steps would reuse the existing steps from the rollout config. Without steps, progressive and all at once modes would behave the same way, right?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation is the same as for rollout, so there cannot be a no-step progressive strategy (done in the webhook). "It could be reasonable to assume that choosing the progressive rollout mode without also adding steps would reuse the existing steps from the rollout config." I would not assume this, it seems like a nasty side-effect, but if you think it's a good way forward, I can implement it.

Comment thread api/v1alpha1/worker_types.go Outdated
Comment thread internal/planner/planner.go
Comment thread api/v1alpha1/worker_types.go
Comment on lines +60 to +61
if s.RollbackStrategy == nil {
s.RollbackStrategy = &RollbackStrategy{Strategy: RollbackAllAtOnce}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a change in behavior that should be called out in release notes.

Is there a way to disable rollbacks and keep the existing behavior of treating every version change as a new rollout? This should be documented as well.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it's possible to disable the rollback by setting the max version age to 0. I've also updated the docs.

Comment thread api/v1alpha1/worker_types.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Rollback mode

3 participants